Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: fix issue with blocks within another blocks #351

Merged
merged 3 commits into from
Apr 8, 2022

Conversation

martinfleis
Copy link
Member

Fixing the case when one block is fully within the exterior of the other. It currently creates overlapping blocks, this should fix it.

@martinfleis martinfleis requested a review from jGaboardi April 8, 2022 21:30
Comment on lines -651 to +638
predicate="intersects",
predicate="within",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

within is faster and with point in polygon results in the same thing

Comment on lines -671 to -673
blocks[blocks.geometry.name] = gpd.GeoSeries(
pygeos.polygons(blocks.exterior.values.data), crs=blocks.crs
)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally did this to cleanup interior after complex dissolve but it seems that GEOS got much better on it so it may not be needed. This is what was causing the issue in the first place, so if we want some cleaning procedure, it needs to be different.

Comment on lines 684 to 690
bl_ID_to_uID = centroids_w_bl_ID2[[unique_id, id_name]]

buildings_m = buildings[[unique_id]].merge(
bl_ID_to_uID, on=unique_id, how="left"
)
self.buildings_id = buildings_m[id_name]
self.buildings_id.index = self.buildings.index
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a simplification.

@codecov
Copy link

codecov bot commented Apr 8, 2022

Codecov Report

Merging #351 (2024927) into main (b347429) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #351      +/-   ##
==========================================
- Coverage   95.87%   95.87%   -0.01%     
==========================================
  Files          13       13              
  Lines        2815     2811       -4     
==========================================
- Hits         2699     2695       -4     
  Misses        116      116              
Impacted Files Coverage Δ
momepy/elements.py 96.75% <100.00%> (-0.05%) ⬇️
momepy/shape.py 91.52% <100.00%> (ø)
momepy/weights.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b347429...2024927. Read the comment docs.

@jGaboardi
Copy link
Member

@martinfleis Should we add a codecov.yml so that we don't get these failing checks from codecov? If yes, I can put in the PR.

@martinfleis
Copy link
Member Author

Should we add a codecov.yml so that we don't get these failing checks from codecov?

Yes. We should also drop python 3.7, can you include that bump in the same one?

@jGaboardi
Copy link
Member

Should we add a codecov.yml so that we don't get these failing checks from codecov?

Yes. We should also drop python 3.7, can you include that bump in the same one?

Sure.

@jGaboardi jGaboardi merged commit e862539 into pysal:main Apr 8, 2022
@martinfleis martinfleis deleted the fix_blocks branch April 8, 2022 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants